[grid] Bundle Redis-backed SessionMap by default#17441
Conversation
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Review Summary by QodoBundle Redis-backed SessionMap by default and add configuration support
WalkthroughsDescription• Bundle Redis-backed SessionMap in selenium-server.jar by default • Add configuration flags for sessions scheme and implementation • Fix null startKey handling in RedisBackedSessionMap.get() • Expand test coverage with new SessionMapFlags and edge case tests Diagramflowchart LR
A["SessionMapFlags"] -->|"Add scheme & implementation params"| B["Configuration Support"]
C["RedisBackedSessionMap"] -->|"Fix null startKey bug"| D["Null Safety"]
E["BUILD.bazel"] -->|"Add redis dependency"| F["Default Bundling"]
B --> G["Enhanced SessionMap"]
D --> G
F --> G
H["New Tests"] -->|"SessionMapFlags & edge cases"| G
File Changes1. java/src/org/openqa/selenium/grid/sessionmap/config/SessionMapFlags.java
|
Code Review by Qodo
Context used 1. New Redis tests lack Docker gating
|
|
Does this mean we're increasing the size of the jar file? What is the size difference since we have added this to the distributor as well? We should pay attention to that so we don't bloat the server. |
|
@diemol. Yes, the diff is 42.3 MB...51M. The same size applies to cross components (Distributor, SessionMap, and later SessionQueue). Do you think it's worth it? |
|
Should be OK. |
|
Persistent review updated to latest commit 6237f7e |
|
Persistent review updated to latest commit 4190fd9 |
|
Persistent review updated to latest commit 4b1ad34 |
🔗 Related Issues
💥 What does this PR do?
selenium-session-map-rediswas not bundled in selenium-server.jar by default, requiring users to manually manage it on the classpath - unlike distributor/redis, which has been built-in since [grid] Add Distributor Redis-backed implementation as built-in support #17396. Generic is Redis-backed support as default cross components.🔧 Implementation Notes
🤖 AI assistance
💡 Additional Considerations
🔄 Types of changes